fix(router): Outlet now actually applies its animation configuration - #84
Merged
Conversation
OutletConfig.animate was defined in the type but never read in the implementation — the underlying reconcile call passed only getTargetKeys and renderSlot, so nothing wired the animation config through to the control ctx. Consumers configuring `animate` saw abrupt route transitions regardless of what they set. Outlet now provides AnimationConfigCtx (the same tag when/match/each use) via Effect.provideService, matching the pattern those combinators follow. provideService uses provideContext internally rather than provideSomeLayer's scopedWith — no scope is created and no finalizer race is introduced (see #78 for the finalizer-race pattern we're deliberately avoiding). Also adds an `intro?: boolean` field to OutletConfig, so the initially matched route can re-animate on hydration in cases like a decorative opening scene. Same shape as when/match/each/animated. AnimationConfigCtx and ClientControlCtx are now re-exported from @effex/dom's package root (they were exported from @effex/dom/Control/index.ts but not lifted). Regression test in packages/router/src/Outlet.test.ts verifies that setting `animate` makes AnimationConfigCtx observable from inside a route's render function, and that omitting it keeps the config absent. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The intro-flag hydration tests waited 20ms after `hydrate()` for enter animations to invoke `onBeforeEnter`. That was too tight now that `forkSlotEnter` waits one requestAnimationFrame on the hydration path (introduced in #83 to fix a first-load CSS-application race) — in jsdom rAF is roughly a setTimeout(16), plus queue drain and the subsequent enter lifecycle work. Local runs stayed just under the margin; CI runners occasionally didn't. Bumped the three affected tests to 100ms. Also bumped the "does not re-animate SSR items by default" wait — the assertion is `not toHaveBeenCalled`, so a longer wait protects against late-firing regressions rather than hiding them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Deploying effex-api with
|
| Latest commit: |
275826e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ca16d8c7.effex-api.pages.dev |
| Branch Preview URL: | https://fix-outlet-animation-config.effex-api.pages.dev |
Deploying effex with
|
| Latest commit: |
275826e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://11e866ea.effex.pages.dev |
| Branch Preview URL: | https://fix-outlet-animation-config.effex.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OutletConfig.animatewas defined in the type but never read in the implementation — the underlyingreconcilecall passed onlygetTargetKeysandrenderSlot, so nothing wired the animation config through to the control ctx. Consumers configuringanimatesaw abrupt route transitions regardless of what they set.Outletnow providesAnimationConfigCtx(the same tagwhen/match/eachuse) viaEffect.provideService, matching the pattern those combinators follow.Also adds an
intro?: booleanfield toOutletConfig, so the initially matched route can re-animate on hydration in cases like a decorative opening scene. Same shape aswhen/match/each/animated.Scope-safety note
Effect.provideServiceusesprovideContextinternally rather thanprovideSomeLayer'sscopedWith, so it does not create a fresh scope tied to the effect's lifetime the wayEffect.provide(effect, Layer)does. That's the mechanism behind #78's finalizer race — the subscription fibersreconcileforks inherit the merged context and keep it for their lifetime, even after Outlet's outerEffect.genreturns. Same patternwhen/match/eachhave been using without issue.Also
AnimationConfigCtxandClientControlCtxare now re-exported from@effex/dom's package root (they were exported from@effex/dom/Control/index.tsbut not lifted to the package index).Regression test
packages/router/src/Outlet.test.ts— two tests:animatemakesAnimationConfigCtxobservable from inside a route's render function.animate/introkeeps the config absent (Effect.serviceOptionreturnsOption.none()), so downstream code treats the slot as non-animated.Verified the first test fails against the pre-fix Outlet.
Test plan
packages/router+packages/domsuite: 467 passed, 2 skipped🤖 Generated with Claude Code